Warn about missing license/description in `cargo package`.

It's very bad practice to not have a license in a
published (theoretically) open-source package, since the default
position is all-rights-reserved. Hence, cargo will now warn if this
metadata field is missing from the manifest when creating a package.

Similarly, a lack of description makes using crates.io less nice, since
there's no indication of what a package does other than the name (and
possibly documentation etc. links, but these are often missing too).

These metadata fields are not immediately obvious so `cargo` can be a
little intelligent and provide some hints that they exist.

Closes #902.

diff --git a/src/bin/package.rs b/src/bin/package.rs
index ed981e742ff16d74ae5466f46985ada609e0d9f0..057d5c90f18707484a95c05ca5147c7dee27de32 100644 (file)
--- a/src/bin/package.rs
+++ b/src/bin/package.rs
@@ -8,6 +8,7 @@ struct Options {
     flag_verbose: bool,
     flag_manifest_path: Option<String>,
     flag_no_verify: bool,
+    flag_no_metadata: bool,
     flag_list: bool,
 }
 
@@ -21,6 +22,7 @@ Options:
     -h, --help              Print this message
     -l, --list              Print files included in a package without making one
     --no-verify             Don't verify the contents by building them
+    --no-metadata           Ignore warnings about a lack of human-usable metadata
     --manifest-path PATH    Path to the manifest to compile
     -v, --verbose           Use verbose output
 
@@ -31,7 +33,8 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult<Option<()>
     let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path));
     ops::package(&root, shell,
                  !options.flag_no_verify,
-                 options.flag_list).map(|_| None).map_err(|err| {
+                 options.flag_list,
+                 !options.flag_no_metadata).map(|_| None).map_err(|err| {
         CliError::from_boxed(err, 101)
     })
 }

diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
index 667b5a4eb69636ae91e16f28dda482a0b4888ece..25bdfd7143cf709cc9244ac1f192fb37aaee5f42 100644 (file)
--- a/src/cargo/ops/cargo_package.rs
+++ b/src/cargo/ops/cargo_package.rs
@@ -26,11 +26,16 @@ impl Drop for Bomb {
 pub fn package(manifest_path: &Path,
                shell: &mut MultiShell,
                verify: bool,
-               list: bool) -> CargoResult<Option<Path>> {
+               list: bool,
+               metadata: bool) -> CargoResult<Option<Path>> {
     let mut src = try!(PathSource::for_path(&manifest_path.dir_path()));
     try!(src.update());
     let pkg = try!(src.get_root_package());
 
+    if metadata {
+        try!(check_metadata(&pkg, shell));
+    }
+
     if list {
         let root = pkg.get_manifest_path().dir_path();
         let mut list: Vec<_> = try!(src.list_files(&pkg)).iter().map(|file| {
@@ -62,6 +67,40 @@ pub fn package(manifest_path: &Path,
     Ok(Some(bomb.path.take().unwrap()))
 }
 
+// check that the package has some piece of metadata that a human can
+// use to tell what the package is about.
+fn check_metadata(pkg: &Package, shell: &mut MultiShell) -> CargoResult<()> {
+    let md = pkg.get_manifest().get_metadata();
+
+    let mut missing = vec![];
+
+    macro_rules! lacking {
+        ($($field: ident),*) => {{
+            $(
+                if md.$field.as_ref().map_or(true, |s| s.is_empty()) {
+                    missing.push(stringify!($field))
+                }
+                )*
+        }}
+    }
+    lacking!(description, license)
+
+    if !missing.is_empty() {
+        let mut things = missing.slice_to(missing.len() - 1).connect(", ");
+        // things will be empty if and only if length == 1 (i.e. the only case to have no `or`).
+        if !things.is_empty() {
+            things.push_str(" or ");
+        }
+        things.push_str(*missing.last().unwrap());
+
+        try!(shell.warn(
+            format!("Warning: manifest has no {things}. \
+                    See http://doc.crates.io/manifest.html#package-metadata for more info.",
+                    things = things).as_slice()))
+    }
+    Ok(())
+}
+
 fn tar(pkg: &Package, src: &PathSource, shell: &mut MultiShell,
        dst: &Path) -> CargoResult<()> {
 

diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs
index 02157c4063a8d6d41f28c463df0bab193bcea5cc..37bb22a912473c984fe859877724dd3f6f575732 100644 (file)
--- a/src/cargo/ops/registry.rs
+++ b/src/cargo/ops/registry.rs
@@ -32,8 +32,9 @@ pub fn publish(manifest_path: &Path,
     let (mut registry, reg_id) = try!(registry(shell, token, index));
     try!(verify_dependencies(&pkg, &reg_id));
 
-    // Prepare a tarball
-    let tarball = try!(ops::package(manifest_path, shell, verify, false)).unwrap();
+    // Prepare a tarball, with a non-surpressable warning if metadata
+    // is missing since this is being put online.
+    let tarball = try!(ops::package(manifest_path, shell, verify, false, true)).unwrap();
 
     // Upload said tarball to the specified destination
     try!(shell.status("Uploading", pkg.get_package_id().to_string()));

diff --git a/src/doc/manifest.md b/src/doc/manifest.md
index 757183370d087eb32f9e249ccc81d0c2ce98ad97..b98dbcf63400c876a3da24139a40ee971ff75829 100644 (file)
--- a/src/doc/manifest.md
+++ b/src/doc/manifest.md
@@ -96,6 +96,12 @@ keywords = ["...", "..."]
 license = "..."
 ```
 
+The [crates.io](https://crates.io) registry will render the description, display
+the license, link to the three URLs and categorize by the keywords. These keys
+provide useful information to users of the registry and also influence the
+search ranking of a crate. It is highly discouraged to omit everything in a
+published crate.
+
 
 # The `[dependencies.*]` Sections
 

diff --git a/tests/support/mod.rs b/tests/support/mod.rs
index ff61bb06f199f7dcbfbb98e2e99fc8604b136a46..36532ffdd8577d215bcac5ba58f1ac9b5bdfd2a9 100644 (file)
--- a/tests/support/mod.rs
+++ b/tests/support/mod.rs
@@ -516,3 +516,4 @@ pub static PACKAGING:   &'static str = "   Packaging";
 pub static DOWNLOADING: &'static str = " Downloading";
 pub static UPLOADING:   &'static str = "   Uploading";
 pub static VERIFYING:   &'static str = "   Verifying";
+pub static WARNING:     &'static str = "     Warning";

diff --git a/tests/test_cargo_package.rs b/tests/test_cargo_package.rs
index c4a07ba27af5dcccc6137ca5912a2f0bfd48a424..4c6b68b4ef8f1dfa7aa4d79739334fadeb198471 100644 (file)
--- a/tests/test_cargo_package.rs
+++ b/tests/test_cargo_package.rs
@@ -4,7 +4,7 @@ use tar::Archive;
 use flate2::reader::GzDecoder;
 
 use support::{project, execs, cargo_dir, ResultTest};
-use support::{PACKAGING, VERIFYING, COMPILING};
+use support::{PACKAGING, WARNING, VERIFYING, COMPILING};
 use hamcrest::{assert_that, existing_file};
 
 fn setup() {
@@ -18,6 +18,8 @@ test!(simple {
             version = "0.0.1"
             authors = []
             exclude = ["*.txt"]
+            license = "MIT"
+            description = "foo"
         "#)
         .file("src/main.rs", r#"
             fn main() { println!("hello"); }
@@ -55,3 +57,76 @@ src[..]main.rs
                 "unexpected filename: {}", f.filename())
     }
 })
+
+test!(metadata_warning {
+    let p = project("all")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/main.rs", r#"
+            fn main() {}
+        "#);
+    assert_that(p.cargo_process("package"),
+                execs().with_status(0).with_stdout(format!("\
+{packaging} foo v0.0.1 ({dir})
+{verifying} foo v0.0.1 ({dir})
+{compiling} foo v0.0.1 ({dir}[..])
+",
+        packaging = PACKAGING,
+        verifying = VERIFYING,
+        compiling = COMPILING,
+        dir = p.url()).as_slice())
+                .with_stderr("Warning: manifest has no description or license. See \
+                              http://doc.crates.io/manifest.html#package-metadata for more info."));
+
+    let p = project("one")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+        "#)
+        .file("src/main.rs", r#"
+            fn main() {}
+        "#);
+    assert_that(p.cargo_process("package"),
+                execs().with_status(0).with_stdout(format!("\
+{packaging} foo v0.0.1 ({dir})
+{verifying} foo v0.0.1 ({dir})
+{compiling} foo v0.0.1 ({dir}[..])
+",
+        packaging = PACKAGING,
+        verifying = VERIFYING,
+        compiling = COMPILING,
+        dir = p.url()).as_slice())
+                .with_stderr("Warning: manifest has no description. See \
+                              http://doc.crates.io/manifest.html#package-metadata for more info."));
+
+    let p = project("both")
+        .file("Cargo.toml", format!(r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+        "#))
+        .file("src/main.rs", r#"
+            fn main() {}
+        "#);
+    assert_that(p.cargo_process("package"),
+                execs().with_status(0).with_stdout(format!("\
+{packaging} foo v0.0.1 ({dir})
+{verifying} foo v0.0.1 ({dir})
+{compiling} foo v0.0.1 ({dir}[..])
+",
+        packaging = PACKAGING,
+        verifying = VERIFYING,
+        compiling = COMPILING,
+        dir = p.url()).as_slice()));
+
+})

diff --git a/tests/test_cargo_publish.rs b/tests/test_cargo_publish.rs
index 5d2d7f7bd29c238cb649d65f7e76396d216c1509..edd3d237e95b4655a8fad34588ccdd2256a4dc26 100644 (file)
--- a/tests/test_cargo_publish.rs
+++ b/tests/test_cargo_publish.rs
@@ -41,6 +41,8 @@ test!(simple {
             name = "foo"
             version = "0.0.1"
             authors = []
+            license = "MIT"
+            description = "foo"
         "#)
         .file("src/main.rs", "fn main() {}");
 
@@ -82,6 +84,8 @@ test!(git_deps {
             name = "foo"
             version = "0.0.1"
             authors = []
+            license = "MIT"
+            description = "foo"
 
             [dependencies.foo]
             git = "git://path/to/nowhere"
@@ -102,6 +106,8 @@ test!(path_dependency_no_version {
             name = "foo"
             version = "0.0.1"
             authors = []
+            license = "MIT"
+            description = "foo"
 
             [dependencies.bar]
             path = "bar"

diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs
index 5ae85d6f54addd9b3be01ae33ba361cdfeee449e..3eb5f982ebb68534fcd74e82bfbbcff53f69f871 100644 (file)
--- a/tests/test_cargo_registry.rs
+++ b/tests/test_cargo_registry.rs
@@ -175,6 +175,8 @@ test!(package_with_path_deps {
             name = "foo"
             version = "0.0.1"
             authors = []
+            license = "MIT"
+            description = "foo"
 
             [dependencies.notyet]
             version = "0.0.1"